Skip to content

GitHub Issue 903: Remove Cross-Container Sample and Data Class Import Feature#7698

Merged
XingY merged 16 commits into
developfrom
fb_dropCrossFolder
Jun 2, 2026
Merged

GitHub Issue 903: Remove Cross-Container Sample and Data Class Import Feature#7698
XingY merged 16 commits into
developfrom
fb_dropCrossFolder

Conversation

@XingY
Copy link
Copy Markdown
Contributor

@XingY XingY commented May 26, 2026

Rationale

This PR removes the support for cross-container sample and data class import feature. The feature allowed import of samples and data class rows belonging to other folders in a single file, which was implemented via temp partitioned files, which is error-prone. Attempts to update/merge rows that does not exist in the current container now generate a generic error message ("Sample/Data does not exist in : "), without revealing its existence in other folders.

Related Pull Requests

Changes

  • Fixed issue that orphaned exp.material record is created when merging a sample in a container that user lacks read permission to
  • Remove crossFolderImport from DataIteratorContext and AbstractQueryImportAction.Params
  • Remove stale QueryUpdateService.hasExistingRowsInOtherContainers
  • Remove DefaultQueryUpdateService.configureCrossFolderImport
  • Remove cross-folder partitioning, folder-column detection, and per-container split-file logic from MultiDataTypeCrossProjectDataIterator
  • Container/Folder columns are now always dropped during sample and data class import (previously they were conditionally kept for cross-folder merges)
  • SampleTypeUpdateServiceDI.getExistingRows: use ElevatedUser when searching other containers for cross-container existence checks, so samples are not missed due to the requesting user's limited read permissions
  • ExpDataClassDataTableImpl._select: add ClassId to the RowId filter to avoid returning a row from a different data class with the same RowId; add fallback lookup against exp.data when allowCrossContainer is true to account for data filtered out due to user's permission
  • Standardize "not found" error messages across samples and data classes for data present in an folder without permission and data don't exist anywhere

Comment thread api/src/org/labkey/api/query/AbstractQueryUpdateService.java Outdated
Comment thread api/src/org/labkey/api/query/DefaultQueryUpdateService.java
Comment thread experiment/src/client/test/integration/AssayDesignCrud.ispec.ts
Comment thread experiment/src/org/labkey/experiment/ExpDataIterators.java
Comment thread experiment/src/org/labkey/experiment/ExpDataIterators.java

if (dataRow == null && allowCrossContainer)
{
// data not found from queryTable but exist in exp.data, which happens when users lack of permission to data's container
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite follow. Shouldn't the row in the queryTable's container always align with the container of the exp.data? Isn't it the same thing, hence why we don't have a container column on the provisioned table?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queryTable is a filtered ContainerFilterable table with container filter for the DataClassData table, that checks for user's permission. ExperimentService.get().getTinfoData()returns a SchemaTableInfo, that doesn't care about user, nor container.

Comment thread experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java Outdated
@XingY XingY requested a review from labkey-nicka June 2, 2026 01:13
@XingY XingY merged commit 8e2e605 into develop Jun 2, 2026
8 of 13 checks passed
@XingY XingY deleted the fb_dropCrossFolder branch June 2, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants